Document .descendants and .subclasses advice#356
Document .descendants and .subclasses advice#356alexanderadam wants to merge 1 commit intogithub:mainfrom
.descendants and .subclasses advice#356Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds guidance to the styleguide discouraging the use of Class#descendants due to reliability issues. The addition documents that Class#descendants is non-deterministic because it doesn't account for classes that haven't been autoloaded yet and can behave inconsistently with garbage collection, particularly in tests with dynamically defined classes.
- Documents the unreliability of
Class#descendantswith autoloading and garbage collection - Provides a code example demonstrating the problematic pattern
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
STYLEGUIDE.md
Outdated
| Avoid using `Class#descendants` as it is unreliable. | ||
| It doesn't know about classes that haven't been autoloaded yet, and it's non-deterministic with regards to garbage collection of classes. | ||
| In tests that dynamically define classes, GC timing can mean `Class#descendants` may or may not include those dynamically defined classes. |
There was a problem hiding this comment.
[nitpick] The documentation should provide an alternative approach or workaround for scenarios where developers might need to track subclasses. Without suggesting alternatives, developers may be unclear on what to use instead of Class#descendants.
0740e10 to
0c4848b
Compare
Class.descendants advice.descendants and .subclasses advice
A wise man, let's call him Ben, once said:
And I totally trust him on that because he's making a… GoodJob 🥁
This is generally unwanted to be documented in Rails.
But I also understand Ben's reasoning.
Now the question is: do you want to have it here?
Shall I add the cops (1, 2) here? 🤔
PS: I'm looking for a new adventure in case anybody is looking to hire or work with a Ruby/Rails/Crystal dev